e2e/davinci-app/components/qr-code.ts (1)

10-14: Consider adding a data-testid to the error element for e2e test coverage.

This would enable testing error scenarios in automated tests.

🧪 Proposed improvement

   if (collector.error) {
     const errorEl = document.createElement('p');
     errorEl.innerText = `QR Code error: ${collector.error}`;
+    errorEl.setAttribute('data-testid', 'qr-code-error');
     formEl.appendChild(errorEl);
     return;
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-app/components/qr-code.ts` around lines 10 - 14, Add a
data-testid attribute to the error paragraph so e2e tests can select it: when
handling collector.error in the QR code component (the branch that creates
errorEl and appends to formEl), set errorEl.dataset.testid = 'qr-code-error' (or
use errorEl.setAttribute('data-testid', 'qr-code-error')) before appending; keep
the existing innerText and return behavior unchanged.

e2e/davinci-app/components/qr-code.ts (1)

20-23: Validate QR image payload format before assigning img.src.

Line 20 trusts collector.output.src directly; add a data-URI guard so malformed/hosted URLs are rejected explicitly.

Proposed hardening

+  const isDataImageUri = /^data:image\/[a-zA-Z+.-]+;base64,/.test(collector.output.src);
+  if (!isDataImageUri) {
+    const errorEl = document.createElement('p');
+    errorEl.innerText = 'QR Code error: invalid QR image payload';
+    formEl.appendChild(errorEl);
+    return;
+  }
+
   const img = document.createElement('img');
   img.src = collector.output.src;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-app/components/qr-code.ts` around lines 20 - 23, Validate that
collector.output.src is a proper data URI before assigning it to img.src: in the
QR component where img.src = collector.output.src is set, add a guard that
checks the string starts with "data:image/" and matches a data URI pattern
(e.g., /^data:image\/[a-zA-Z]+;base64,/) and only then assign to img.src; if
validation fails, do not set img.src and instead set a safe fallback (e.g.,
leave alt text, set a placeholder src or log/emit an error) prior to
container.appendChild(img) so malformed or hosted URLs are explicitly rejected.

e2e/davinci-suites/src/qr-code.test.ts (1)

21-21: Replace generic requestfinished waits with state-specific assertions.

Lines 21, 26, 30, and 35 can be flaky because any request completion may satisfy them. Wait for the next expected UI state (or specific response URL) after each action.

Also applies to: 26-26, 30-30, 35-35

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-suites/src/qr-code.test.ts` at line 21, Replace the flaky
page.waitForEvent('requestfinished') calls with state-specific waits: after each
action that currently uses page.waitForEvent('requestfinished') (instances of
page.waitForEvent('requestfinished') in the test), wait for the specific network
response using page.waitForResponse(resp =>
resp.url().includes('<expected-path-or-endpoint>') && resp.status() === 200) or
wait for the expected UI state via await
page.waitForSelector('<expected-selector>', { state: 'visible' }) (choose the
response URL or selector that matches the next UI change for each of the four
occurrences).

e2e/davinci-suites/src/qr-code.test.ts (2)

47-52: Conditional fallback assertion may silently pass if fallback is missing.

The conditional if (fallbackVisible) means the test will pass even if the fallback element is never rendered. If the fallback text is expected behavior for this flow (the PR screenshot shows "04AMFGOEWFENIB62O" as a manual code), consider making this assertion unconditional:

  const fallback = page.locator('[data-testid="qr-code-fallback"]');
- const fallbackVisible = await fallback.isVisible();
- if (fallbackVisible) {
-   const fallbackText = await fallback.textContent();
-   expect(fallbackText).toBeTruthy();
- }
+ await expect(fallback).toBeVisible();
+ const fallbackText = await fallback.textContent();
+ expect(fallbackText).toBeTruthy();

If the fallback is truly optional depending on the flow configuration, consider adding a comment explaining when it's expected vs. optional.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-suites/src/qr-code.test.ts` around lines 47 - 52, The current
conditional check around the QR fallback allows the test to silently pass when
the fallback element is not present; update the test so that the fallback text
assertion is unconditional (remove the `if (fallbackVisible)` guard) and
directly assert the fallback element is visible and has non-empty text using the
existing `fallback`, `fallbackVisible`/`fallbackText` variables (e.g., ensure
you first assert `fallback.isVisible()` then `fallback.textContent()` is
truthy), or if the fallback is legitimately optional add a clear comment
explaining when it is expected and keep the conditional but assert visibility
state explicitly.

---

20-26: Consider more targeted request waiting to reduce flakiness.

Using page.waitForEvent('requestfinished') will resolve on any completed network request, not necessarily the one triggered by the preceding action. If the page has background requests (analytics, polling, etc.), this could introduce intermittent failures or false positives.

Consider using page.waitForResponse() with a URL pattern, or wrapping the click + wait in Promise.all:

await Promise.all([
  page.waitForResponse((resp) => resp.url().includes('/expected-endpoint') && resp.ok()),
  page.getByRole('button', { name: 'USER_LOGIN' }).click(),
]);

This pattern is more resilient and explicitly ties the wait to the expected request.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@e2e/davinci-suites/src/qr-code.test.ts` around lines 20 - 26, Replace the
broad page.waitForEvent('requestfinished') calls around the login actions with
targeted waits tied to the expected network request(s): wrap the click on
page.getByRole('button', { name: 'USER_LOGIN' }).click() and the Sign On click
in Promise.all paired with page.waitForResponse or page.waitForRequest using a
URL predicate that matches the login endpoint (e.g.,
resp.url().includes('/expected-endpoint') && resp.ok()), and do the same for the
second click on page.getByRole('button', { name: 'Sign On' }).click() so the
test explicitly waits for the correct request/response instead of any finished
network request.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes